Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace video player by ExoPlayer #8132

Merged
merged 1 commit into from
Jun 9, 2021
Merged

Replace video player by ExoPlayer #8132

merged 1 commit into from
Jun 9, 2021

Conversation

tobiasKaminsky
Copy link
Member

@tobiasKaminsky tobiasKaminsky commented Mar 8, 2021

Fix #5917
Fix #5800
Fix #4204
Fix #3601
Fix #7647
Fix #3061
Fix #3064

TODO:

Built in video player is known to having problems with streaming, buffering, etc.
Exoplayer should perform better: https://github.com/google/ExoPlayer

I invite every user reporting streaming/playback issues to test this via provided apk.

Signed-off-by: tobiasKaminsky tobias@kaminsky.me

Testing

Writing tests is very important. Please try to write some tests for your PR.
If you need help, please do not hesitate to ask in this PR for help.

unit tests
instrumented tests
UI tests

  • Tests written, or not not needed

@github-actions
Copy link

github-actions bot commented Mar 8, 2021

To potential test user:
This is "only" about testing streaming, playing of files, testing codec, etc.
Known issue right now is that it does not stop playing, even when closing the app.
Force closing the app should help in this case.

All in all feedback sounds promising, so I'll go this way, and generate a new test version.

EDIT: I removed the test file, as many testers replied, thanks for that!
Once there is a new version to test I'll inform you again.

@lftsy
Copy link

lftsy commented Mar 16, 2021

I am interested in alpha testing the apk too with the new player, we just reuploaded our full nextcloud to reset old encryption, and video playing on android is still buggy. Working perfectly on Chrome/Firefox Mobile
Where can we find the apk to test with Exoplayer ? I could provide feedback or adb debugs too
Best Regards

@tobiasKaminsky
Copy link
Member Author

Where can we find the apk to test with Exoplayer ?

I removed it for now, but will update this PR soon and then there is a new version to test.

@kvn1351
Copy link

kvn1351 commented Apr 1, 2021

Any updates on this?

@szaimen
Copy link
Contributor

szaimen commented May 5, 2021

@tobiasKaminsky any update here? :)

@jakobroehrl
Copy link

Working very nice, videos are starting instandly

@szaimen
Copy link
Contributor

szaimen commented May 6, 2021

Working very nice, videos are starting instandly

For me the same, also with local external storage on the server side.

@lftsy
Copy link

lftsy commented May 7, 2021

Thanks, I tested it and it starts indeed smoothly, and changing orientation of the phone is also working smoothly, BUT, I do cope with 3 problems

  • Fullscreen: How can we enable fullscreen ?
  • Crash : When trying to use the back button to exit the movie, it leaves the app
  • Crash: If I try to browse the movie, it also crashes

But thanks for your effort, it is already super nice to see the movies starting so quickly !

Best Regards

@nextcloud nextcloud deleted a comment from github-actions bot May 10, 2021
@tobiasKaminsky
Copy link
Member Author

@lftsy thanks for your feedback.
Can you check newest apk and give us a stracktrace, when it crashes:
https://github.com/nextcloud/android/blob/master/README.md#getting-debug-info-via-logcat

Fullscreen is for now shown in option menu.

@nextcloud-android-bot
Copy link
Collaborator

Codacy

Lint

TypemasterPR
Warnings232231
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings51
Internationalization Warnings9
Multithreaded correctness Warnings9
Performance Warnings72
Security Warnings40
Dodgy code Warnings98
Total306

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings51
Internationalization Warnings9
Multithreaded correctness Warnings9
Performance Warnings72
Security Warnings40
Dodgy code Warnings98
Total306

@nextcloud-android-bot
Copy link
Collaborator

master-IT test failed, but no output was generated. Maybe a preliminary stage failed.

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

stable-IT test failed, but no output was generated. Maybe a preliminary stage failed.

@nextcloud-android-bot
Copy link
Collaborator

@jakobroehrl
Copy link

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/8132.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

"Stream with" out of the player menu doesn't have any effect.

@szaimen
Copy link
Contributor

szaimen commented May 11, 2021

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/8132.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@tobiasKaminsky MKV videos play but you only see a black screen and only hear the audio. I can share the file via mail if you want.

@thibaultmol
Copy link

Will this pullrequest also make it possible to cast content to chromecast and such?
https://medium.com/android-news/sending-media-to-chromecast-has-never-been-easier-c331eeef1e0a

@jakobroehrl
Copy link

Will this pullrequest also make it possible to cast content to chromecast and such?
https://medium.com/android-news/sending-media-to-chromecast-has-never-been-easier-c331eeef1e0a

I don't think. But you can very easy use cromecast if you stream to vlc, bubbleupnp oder allcast

@lftsy
Copy link

lftsy commented May 17, 2021

Good evening everybody and sorry for the delay in answering

** Actions**

  1. I have tested your last version @tobiasKaminsky and I am not able to reproduce the 2 crashes I had browsing the movie and using the back button ! And I also see that arrows keys to browse the movie do not appear twice
    => Thanks for the fixes !!
  2. Fullscreen is also working indeed from the option menu, sorry if I missed this one on the older version !

** Problems I coped with**

  1. Audio in bg: I did cope several times with the "Known issue right now is that it does not stop playing, even when closing the app."
    => Let me know if you need an output of this one ?
  2. Playing old .avi not working and no error displayed : I was tying to play an old .avi file Fantomas.avi and on the app, it doesn't start playing and do not display any error, I do see "" in the adb logcat "Unsupported RIFF format: 1096173856" + "Playback error" , cf adb-logcat-nextcloud-qa-8132-avi-not-playing-no-error-visible.log
    => when there is an error, it might be helpful here to display a generic error that format is not supported

@ALL

  1. Thank you very much for your work on this problem, I will keep testing it and provide steps to reproduce + logcat if I cope with other problems related to ExoPlayer integration

Best Regards

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
@tobiasKaminsky
Copy link
Member Author

I'll merge this now, after CI.

@lftsy thanks for your feedback, can you create new issues once this happen again?

@github-actions
Copy link

github-actions bot commented Jun 2, 2021

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/8132.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

@tobiasKaminsky tobiasKaminsky merged commit 18594f9 into master Jun 9, 2021
@delete-merged-branch delete-merged-branch bot deleted the exoPlayer branch June 9, 2021 09:16
@szaimen
Copy link
Contributor

szaimen commented Jun 9, 2021

🎉

@AndyScherzinger AndyScherzinger added this to the Nextcloud App 3.17.0 milestone Jun 9, 2021
@nextcloud nextcloud deleted a comment from gkjcv Jun 10, 2021
@BartAgterbosch
Copy link

Honestly exoplayer is way worse than the original player, at least with the original player when it was unable to process either the video format or the audio format, it would stop player when you closed it, exoplayer just starts spinning up processes like its' life depends on it, is there really no better alternative than this? Video player is essentially broken 99% of the time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment